feat: wire aux sends to audio engine with pre/post fader toggle#1039
feat: wire aux sends to audio engine with pre/post fader toggle#1039
Conversation
Sends previously existed only in the data model and UI — the mixer send sliders had no audio effect. This commit wires sends end-to-end: - Add ReturnTrackNode class (simplified channel strip for return tracks) - Add send tap points to TrackNode (dual-gain approach: pre and post fader gain nodes, toggle via gain ramp for click-free switching) - Wire sends in AudioEngine.syncSends() during playback and live updates - Add preFader field to Send interface with PRE/POST toggle in mixer UI - Render return track channel strips (teal accent) between tracks and master - Support return track level metering in LevelMeter component Closes #984 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes aux sends functional by wiring track sends into the audio engine, adds return-track channel strips (with metering) to the mixer UI, and introduces a PRE/POST fader toggle for each send.
Changes:
- Add
preFader?: booleantoSendand update the store/UI to support PRE/POST toggling. - Implement Web Audio routing for sends and return tracks via
AudioEngine.syncSends(),TrackNodesend tap points, and a newReturnTrackNode. - Render return track channel strips in the mixer and enable return-track metering.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/useTransportScrubLifecycle.test.tsx | Adds syncSends to engine mock used by transport scrub lifecycle tests. |
| src/types/project.ts | Extends Send with preFader?: boolean. |
| src/store/projectStore.ts | Updates updateTrackSend to optionally set preFader and default it on new sends. |
| src/hooks/useTransport.ts | Calls engine.syncSends() on playback start and during live playback updates. |
| src/hooks/tests/useTransport.strudel.test.ts | Adds syncSends to the engine mock. |
| src/engine/tests/TrackNode.sends.test.ts | New unit tests for TrackNode send wiring and pre/post behavior. |
| src/engine/tests/ReturnTrackNode.test.ts | New unit tests for ReturnTrackNode behavior (volume/pan/effects splice/metering). |
| src/engine/TrackNode.ts | Adds dual pre/post send gains, send lifecycle methods, and latency-comp interaction. |
| src/engine/ReturnTrackNode.ts | Introduces a simplified channel strip for return tracks with metering and effect splicing. |
| src/engine/AudioEngine.ts | Adds return track node management + syncSends() wiring logic and metering APIs. |
| src/components/mixer/MixerPanel.tsx | Adds PRE/PST toggle UI per send slot and renders return track strips. |
| src/components/mixer/LevelMeter.tsx | Adds returnTrackId support for metering return tracks. |
| .llm/research/mixer-routing-gap.md | Adds a routing gap analysis doc and references patterns from openDAW. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // 3. Wire sends for each track | ||
| // Build a set of currently active send connections to detect stale ones | ||
| const activeSends = new Map<string, Set<string>>(); // trackId → Set<returnTrackId> | ||
|
|
||
| for (const track of tracks) { | ||
| const trackNode = this.trackNodes.get(track.id); | ||
| if (!trackNode) continue; | ||
|
|
||
| const sends = track.sends ?? []; | ||
| const activeReturnIds = new Set<string>(); | ||
|
|
||
| for (const send of sends) { | ||
| if (!returnTrackIds.has(send.returnTrackId)) continue; | ||
| if (send.amount <= 0) continue; | ||
|
|
||
| const returnNode = this.returnTrackNodes.get(send.returnTrackId); | ||
| if (!returnNode) continue; | ||
|
|
||
| activeReturnIds.add(send.returnTrackId); | ||
| trackNode.connectSend(send.returnTrackId, returnNode.inputGain, send.amount, send.preFader ?? false); | ||
| } | ||
|
|
||
| activeSends.set(track.id, activeReturnIds); | ||
| } | ||
|
|
||
| // 4. Disconnect sends that are no longer active | ||
| // (connectSend already disconnects existing before reconnecting, so this handles | ||
| // the case where a send was removed entirely from the track's sends array) | ||
| // TrackNode.connectSend calls disconnectSend first, so reconnections are safe. | ||
| // We only need to handle sends that were removed from the array: | ||
| for (const track of tracks) { | ||
| const trackNode = this.trackNodes.get(track.id); | ||
| if (!trackNode) continue; | ||
| const active = activeSends.get(track.id) ?? new Set(); | ||
| // Note: TrackNode doesn't expose its sendGains keys, so we rely on | ||
| // connectSend's built-in disconnect-before-connect behavior. | ||
| // For sends with amount=0, explicitly disconnect: | ||
| for (const send of (track.sends ?? [])) { | ||
| if (send.amount <= 0 && !active.has(send.returnTrackId)) { | ||
| trackNode.disconnectSend(send.returnTrackId); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
syncSends() does not reliably disconnect sends that were removed from a track. When a send is deleted from track.sends (e.g. amount set to 0 and the entry is spliced out), the removed returnTrackId is no longer iterated, so TrackNode.disconnectSend() is never called and the old send remains wired. This can leave stale connections/memory and can keep feeding a return until the engine is rebuilt. Consider reconciling against the TrackNode’s currently-connected send IDs (expose them), tracking previous state in AudioEngine, or using trackNode.disconnectAllSends() and then re-adding the active ones each sync.
| for (const track of tracks) { | ||
| const trackNode = this.trackNodes.get(track.id); | ||
| if (!trackNode) continue; | ||
|
|
||
| const sends = track.sends ?? []; | ||
| const activeReturnIds = new Set<string>(); | ||
|
|
||
| for (const send of sends) { | ||
| if (!returnTrackIds.has(send.returnTrackId)) continue; | ||
| if (send.amount <= 0) continue; | ||
|
|
||
| const returnNode = this.returnTrackNodes.get(send.returnTrackId); | ||
| if (!returnNode) continue; | ||
|
|
||
| activeReturnIds.add(send.returnTrackId); | ||
| trackNode.connectSend(send.returnTrackId, returnNode.inputGain, send.amount, send.preFader ?? false); | ||
| } | ||
|
|
There was a problem hiding this comment.
syncSends() calls trackNode.connectSend(...) on every sync pass, and connectSend() always disconnects and recreates the GainNodes. Since useTransport calls syncSends during live parameter updates, this can cause unnecessary audio-graph churn (and potentially clicks because initial gain values are set immediately, not ramped). Prefer creating the send once and then calling trackNode.updateSendAmount(...) when only amount/preFader changes, and only recreate connections when the destination return node changes.
| this.latencyCompNode = null; | ||
| this._reconnectPreFaderSends(); |
There was a problem hiding this comment.
In setLatencyCompensation(samples <= 0), latencyCompNode is set to null before _reconnectPreFaderSends() runs. _reconnectPreFaderSends() then can't disconnect the old delay node from existing pre-fader send taps, leaving the old delay still connected (potential double-routing / phasey summing). Keep a reference to the old delay node and disconnect it from send.pre before nulling, or call _reconnectPreFaderSends() prior to this.latencyCompNode = null with an explicit oldLatencyCompNode to disconnect.
| this.latencyCompNode = null; | |
| this._reconnectPreFaderSends(); | |
| this._reconnectPreFaderSends(); | |
| this.latencyCompNode = null; |
| try { send.pre.disconnect(); } catch { /* noop */ } | ||
| try { send.post.disconnect(); } catch { /* noop */ } | ||
| // Also disconnect the source connections to the gain nodes | ||
| try { this.preFaderOutput.disconnect(send.pre); } catch { /* noop */ } |
There was a problem hiding this comment.
disconnectSend() only calls this.preFaderOutput.disconnect(send.pre), which disconnects from the current pre-fader tap node. If the tap source changed previously (e.g. latency compensation toggled) there may still be an upstream connection from the old source node into send.pre. Consider disconnecting from both possible sources (compressor.disconnect(send.pre) and latencyCompNode?.disconnect(send.pre)) to ensure the send is fully detached.
| try { this.preFaderOutput.disconnect(send.pre); } catch { /* noop */ } | |
| try { this.preFaderOutput.disconnect(send.pre); } catch { /* noop */ } | |
| try { this.compressor.disconnect(send.pre); } catch { /* noop */ } | |
| if (this.latencyCompNode) { | |
| try { this.latencyCompNode.disconnect(send.pre); } catch { /* noop */ } | |
| } |
| spliceEffects(input: AudioNode | null, output: AudioNode | null) { | ||
| try { this.inputGain.disconnect(this.volumeGain); } catch { /* noop */ } | ||
|
|
||
| if (this._effectsOutput) { | ||
| try { this._effectsOutput.disconnect(this.volumeGain); } catch { /* noop */ } | ||
| } | ||
|
|
||
| if (input && output) { | ||
| this.inputGain.connect(input); | ||
| output.connect(this.volumeGain); | ||
| } else { | ||
| this.inputGain.connect(this.volumeGain); | ||
| } | ||
|
|
||
| this._effectsInput = input; | ||
| this._effectsOutput = output; | ||
| } |
There was a problem hiding this comment.
spliceEffects() doesn't disconnect inputGain from the previously spliced effects input (only attempts inputGain.disconnect(volumeGain)). If effects are spliced multiple times or removed (spliceEffects(null, null)), inputGain can remain connected to the old effects input and the restored direct path, causing duplicate routing. Track and disconnect the previous _effectsInput (or call inputGain.disconnect() / inputGain.disconnect(oldInput)) before connecting the new path.
| onClick={() => updateTrackSend(track.id, rt.id, amount || 0.5, !isPreFader)} | ||
| disabled={isFrozen} |
There was a problem hiding this comment.
The PRE/PST toggle click handler uses amount || 0.5, which will unexpectedly jump a send from 0 to 0.5 just by toggling the tap point (audible surprise). If sends are intentionally removed at amount=0, consider disabling the toggle when amount === 0, or changing the store model to persist preFader even when amount is 0 so the toggle doesn’t need to mutate the amount.
| onClick={() => updateTrackSend(track.id, rt.id, amount || 0.5, !isPreFader)} | |
| disabled={isFrozen} | |
| onClick={() => updateTrackSend(track.id, rt.id, amount, !isPreFader)} | |
| disabled={isFrozen || amount === 0} |
|
Triage note: This is a high-value PR (sends were completely cosmetic → now functional). Has merge conflicts after refactoring merges (#1046, #1049, #1050, #1052). Needs rebase. Priority: merge after conflict resolution — this fixes real audio routing functionality. Generated by Claude Code |
Summary
ReturnTrackNodeclass (simplified channel strip for return tracks)TrackNodeusing dual-gain approach (pre + post fader GainNodes, toggle via 5ms gain ramp for click-free switching)AudioEngine.syncSends()during playback start and live parameter updatespreFaderfield toSendinterface with PRE/POST toggle button in mixer UILevelMetercomponentTest plan
npx tsc --noEmit— 0 type errorsnpm test— 2795 pass, 0 fail (24 new send-specific tests)npm run build— succeedsCloses #984
🤖 Generated with Claude Code